Skip to content

Claude-first setup#78

Merged
Dominik1999 merged 10 commits intomainfrom
dominik_add_claude_agents
Apr 22, 2026
Merged

Claude-first setup#78
Dominik1999 merged 10 commits intomainfrom
dominik_add_claude_agents

Conversation

@Dominik1999
Copy link
Copy Markdown
Contributor

Follows: 0xMiden/protocol#2783

Desired workflow:

  1. Point Claude at an issue, ask it to create a PR to address it
  2. Plan
  3. Once plan is approved, Claude works on the implementation
  4. Before a commit, executes a hook to make lint
  5. Before push:
  • runs make test
  • runs two review agents; if any agent comes back with findings, main agent fixes these
  1. Before creating PR, ensures the PR is being created in --draft mode
  2. After PR is created, checks if CHANGELOG.md exists; if yes, proceeds; if not, determines whether it should add an entry or a no changelog label - the main agent takes care of this.
  3. PR author manually reviews the PR; if all looks good, he requests reviews from peers; else explains to Claude what should change and codifies this as skill/hook, so that next time Claude doesn't make the same mistake. As you can see from the commit history, I've had a few iterations on this, but at the end AggLayer: add emergency pause mechanism protocol#2785 is basically one-shotted. We should aim at one-shoting 95% of our PRs (and 5% would result in skills/hooks being updated).
  4. Peers/code owners review the PR. As with the previous point, requested changes (patterns/standards/Miden-specific practices) should be codified.

The goal is to codify our shared knowledge into a set of actionable artifacts.

@WiktorStarczewski WiktorStarczewski self-requested a review April 22, 2026 13:38
Copy link
Copy Markdown
Contributor

@WiktorStarczewski WiktorStarczewski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

Adds Claude Code automation scaffolding: three sub-agents (changelog-manager, code-reviewer, security-reviewer), five shell hooks wired through .claude/settings.json, a /codify-lesson slash command, and a starter CHANGELOG.md. No runtime code changes.

The design is good - I particularly like the two-persona adversarial security reviewer, the parallel code+security review on push, and the deny-by-default draft PR hook. The execution has a handful of real bugs that prevent some hooks from doing what the PR description says they do; I've left inline comments on each.

What's done well

  • Two-persona adversarial security reviewer with merge-and-promote logic - worth stealing.
  • pre-push-review.sh decouples severity policy from agent prompts and keeps it in the hook with a clear comment explaining the divergence.
  • pre-pr-draft.sh is the template the other hooks should aim at: defensive command match, clear --no-draft vs --draft decision, JSON-encoded reason, escape hatch, best-effort corrected-command suggestion.
  • .gitignore switch from ignoring all of .claude/ to ignoring only settings.local.json and worktrees/ is exactly right for shipping shared Claude config.
  • Milestone-driven version resolution for the changelog is a much better design than a hardcoded vX.Y.Z, and the "give up and ask the human" fallback is the right failure mode.

Nits (not worth an inline anchor)

  • Every added file ends with \ No newline at end of file. I would add trailing newlines.
  • pre-pr-draft.sh has trailing whitespace on several lines.
  • .claude/commands/codify-lesson.md - "Touch only .claude/lessons.md without explicit user confirmation" reads backwards; intended meaning seems to be "Touch only lessons.md; hooks and CLAUDE.md edits require confirmation."
  • post-pr-create-changelog.sh uses [ -z X ] || [ -z Y ] || [ -z Z ] && exit 0. It happens to work because &&/|| are equal precedence and left-associate, but it's fragile - I would use an explicit if ... ; then exit 0; fi.
  • CHANGELOG.md style guide says "past-tense imperative" - those are opposites. I think you mean past-tense declarative (Added, Fixed).
  • .claude/CLAUDE.md duplicates a lot of content from the global ~/.claude/CLAUDE.md. Consider narrowing the project file to project-specific deltas.

Suggested minimum bar before merge

  1. Fix the # !/bin/bash shebang or delete pre-push-test.sh.
  2. Delete post-pr-create-ci-monitor.sh or add and wire the ci-monitor agent.
  3. Fix the two gh repo view ... -- <path> commands in changelog-manager.md.
  4. Swap grep -oP -> grep -oE in both post-PR hooks.
  5. Confirm "if" is a real field in your Claude Code version; if not, move filters into matcher or add internal guards.
  6. chmod +x all hook scripts.

Comment thread .claude/hooks/pre-push-test.sh Outdated
Comment thread .claude/hooks/post-pr-create-ci-monitor.sh Outdated
Comment thread .claude/agents/changelog-manager.md Outdated
Comment thread .claude/agents/changelog-manager.md
Comment thread .claude/hooks/post-pr-create-changelog.sh Outdated
Comment thread .claude/settings.json
Comment thread .claude/hooks/post-pr-create-changelog.sh Outdated
Comment thread .claude/hooks/post-pr-create-changelog.sh
Comment thread .claude/hooks/pre-push-review.sh Outdated
Comment thread .claude/hooks/pre-commit-lint.sh
@Dominik1999 Dominik1999 merged commit 5127eb8 into main Apr 22, 2026
12 checks passed
@WiktorStarczewski WiktorStarczewski deleted the dominik_add_claude_agents branch April 22, 2026 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants